build(rsc-runtime): one module graph, drop @modelcontextprotocol/sdk 1.x, expose ./package.json - #571
Merged
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
🦋 Changeset detectedLatest commit: bfbbee8 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
ScriptedAlchemy
added a commit
that referenced
this pull request
Sep 5, 2026
commit: |
…1.x, expose ./package.json
`@agent-bundle/runtime` imported `CallToolResult` from `@modelcontextprotocol/sdk`
1.x for two type positions, which kept the whole 1.x SDK (express 5, hono, jose,
cors, ajv, ...) in every consumer's install. Take the type from
`@modelcontextprotocol/server` 2.x (already a dependency) and publish the exact
result shape the lowerers emit as `McpCallToolResult` / `McpContentBlock`, whose
`_meta` and `structuredContent` are finite JSON objects, so the value stays
assignable to both SDK lines' `CallToolResult`.
Build every public entry in one Rslib lib instead of six bundle-mode libs
stitched with literal-request `output.externals`. Shared modules land in common
chunks once, so each error class is defined once in the dist and `instanceof`
holds across subpaths. The old externals missed `src/state/sqlite.ts`'s direct
`./contract.js` import, so today's tarball carries a second `AgentStateError`
(and three `AgentContractError` / `AgentRequestError`): a sqlite store's
`read({ revision: -1 })` rejection was not `instanceof` the class `./state`
exports. Entry graphs stay disjoint where they must: `node:sqlite` loads only
through `./state/sqlite`, the root and `./plugin` carry no kernel code.
Tests: `state-packaging` now walks the dist import graph per entry, asserts one
definition per error class, and runs a child-process probe that imports every
entry, checks `process.moduleLoadList` for `node:sqlite`, and compares errors
thrown by the sqlite and mount entries against `./state`'s class. A new packed
test installs the release tarball and repeats the probe through the `exports`
map. Both fail against the previous build.
Manifest: `./package.json` export; `@rspack/core` peer marked optional (no
runtime entry imports it; it is `react-server-dom-rspack`'s build-plugin peer).
`react`/`react-dom` stay exact and required: the Flight wire format is pinned to
the React build and `react-server-dom-rspack` loads both at import time.
ScriptedAlchemy
force-pushed
the
build/rsc-runtime-hygiene
branch
from
September 5, 2026 00:33
d5abf58 to
f22c2e9
Compare
…kaging graph tests attachMcpStructuredContent is generic over its input so a CallToolResult typed by either MCP SDK line round-trips instead of being narrowed to McpCallToolResult. The dist-graph tests count class definitions by occurrence, check sqlite confinement by entry reachability, exclude the kernel error classes by definition file, assert every dist file is reached by some entry, run the same static checks against the installed tarball, and strip NODE_OPTIONS from probe children. docs/preview-packages notes the @rspack/core peer is optional.
… compatibility in the example's tests The website MCP page (en/zh) and the package README name the lowered result type and its assignability to both MCP SDK lines. The rsc-agent-runtime example's lowering test annotates a lowered result and the attachMcpStructuredContent round-trip as the 1.x SDK's CallToolResult, so the example's typecheck fails if either narrows again.
ScriptedAlchemy
enabled auto-merge (squash)
September 5, 2026 01:16
…e in lint:release #568 added the gate for the other three publishable packages; the runtime tarball this PR reshapes now goes through the same attw --profile esm-only and scripts/check-declaration-imports.mjs run (0 errors, 0 warnings).
ScriptedAlchemy
added a commit
that referenced
this pull request
Sep 5, 2026
…uperseded by #571's single module graph)
This was referenced Sep 5, 2026
ScriptedAlchemy
added a commit
that referenced
this pull request
Sep 5, 2026
) (#575) * build: compile on one Rspack engine with Rslib 1.0 and Rsbuild 2.2 (#566) Bump @rslib/core to 1.0.0 and @rsbuild/core to 2.2.3 in every workspace package, example, and scaffolder template, and @rstest/* to 0.11.12, so a consumer installs one @rspack/core (2.2.2) and one native binding. Compiler (src/build/rslib.ts): output.autoExternal: false (v1 form); URL and worker parsing off for plugin builds so generated `new URL(…)` and `new Worker(new URL(…))` expressions survive verbatim; inspectConfig runs in production mode with NODE_ENV restored; persistent cache off. externalsType stays Rslib's ESM default, which reproduces the 0.x createRequire shim for CommonJS requires of Node builtins. Own sources resolve sibling modules with fileURLToPath + path.join instead of new URL(…, import.meta.url). Tests: lowered-config and packed assertions that plugin output is self-contained (Node builtins the only externals), a packed guard for a single Rspack engine, and inspect coverage under NODE_ENV=development. * chore: reference #575 in the rslib-1 changeset * fix(build): hand an unset NODE_ENV back after inspecting, even when the inspection throws * docs: name the Rslib 1 option shape (output.autoExternal) in AGENTS.md * docs(types): state the one-engine hatch reality on AgentBundleToolsConfig (Codex review) * chore: drop the runtime chunk-name note from the rslib-1 changeset (superseded by #571's single module graph)
This was referenced Sep 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the
packages/rsc-runtimeitems of #566 (Section 2). Scope is@agent-bundle/runtimeonly; the other publishable packages' manifests and configs are untouched, as issrc/state/sqlite.ts.What changed
P1 —
@modelcontextprotocol/sdk1.x droppedpackages/rsc-runtime/src/lower-mcp.ts:3andsrc/project-mcp.tsimportCallToolResultfrom@modelcontextprotocol/server2.x (already a dependency) instead of@modelcontextprotocol/sdk/types.js;package.json:88no longer lists@modelcontextprotocol/sdk@1.30.0;pnpm-lock.yamlloses the runtime's importer entry (the 1.x snapshot itself stays,agent-bundlestill depends on it).structuredContentasRecord<string, unknown>and_metaas a record, 2.x types both asunknown. Passing the 2.x type straight through brokeexamples/rsc-agent-runtime/src/mcp/handlers.ts, which hands runtime results to a 1.x handler. The package now states the shape it actually emits —McpCallToolResult/McpContentBlock(lower-mcp.ts:51-68, exported from the root atsrc/index.ts:97): SDK content blocks,_metaandstructuredContentas finiteJsonObjects — which is assignable to both SDK lines'CallToolResult.lowerMcpResult,documentToCallToolResult, andMcpProjectedToolResult.resultreturn/carry it;attachMcpStructuredContent(project-mcp.ts:215-228) is generic over its input (<T extends CallToolResult>(result: T, value) => T), so a caller holding a result typed by either SDK line gets it back as the same type — a 1.xCallToolResultis not assignable toMcpCallToolResult(_metais a loose record there), and the first draft of this PR would have stopped such callers compiling (self-review finding 1). Everymodelcontextprotocolimport in the repo (git grep -n "modelcontextprotocol" -- ':!repos' ':!pnpm-lock.yaml') typechecks against it.@modelcontextprotocol/sdk(manifest,dist/*.js,dist/*.d.ts); the shipped declarations import onlyreact,zod,effect,@modelcontextprotocol/server,rsc-markdown-stream.P2 — one module graph, no string externals
packages/rsc-runtime/rslib.config.ts:5-35builds all nine public entries in one bundle-modelib(option (a)). Rslib/Rspack emits every module two entries share in a common chunk (dist/<id>.js) that both import, so each class exists once; a module only one entry reaches stays in that entry's own chunk, so the graphs remain disjoint where they must.Why not (b)
bundle: false+redirect.js— measured, not assumed (scratch build of the same nine entries into/tmp, Rslib 0.23.2,redirect.js.extension: true): it passes every boundary instate-packaging.test.tsand yields an identical identity report, so it is a valid answer too, but it ships 48 JS files instead of 22 at +1.4 % bytes (374,108 vs 368,894), forces fourexportsimporttargets to move (state,notices,mount,lineage→*/index.js, plus two test fixtures), and shows no measurable cold-import difference. Same guarantee, more files and churn, so (a).Evidence (release-mode
npm pack, before =origin/main, after = this branch):dependencies@modelcontextprotocol/sdk1.30.0,@modelcontextprotocol/server2.0.0,effect,flare-redact,react-server-dom-rspack,rsc-markdown-stream,zod@modelcontextprotocol/server2.0.0,effect,flare-redact,react-server-dom-rspack,rsc-markdown-stream,zodpeerDependencies@rspack/core ^2.2.0-0,react 19.2.8,react-dom 19.2.8@rspack/corenowpeerDependenciesMeta.optionaldist/*.jsdist/*.d.tsthis.name = 'AgentStateError'definitions in diststate.js,state/sqlite.js)AgentContractError/AgentRequestErrordefinitionsnode:sqlitereferenced bystate/sqlite.jsonlystate/sqlite.jsonly__webpack_require__in distexportsmap./package.jsonsideEffects: falseattw --profile esm-onlyon the tarballpublinton the tarballrsbuild-plugin-publint,throwOn: 'warning')The duplication in today's build is real, not hypothetical: the literal externals covered
'../state/index.js', butsrc/state/sqlite.tsimports./contract.jsdirectly (expectOperable,expectRevisionShape), so Rspack re-bundled the kernel's contract module — and itsAgentStateError— intostate/sqlite.js. Observable today:The existing
state-packagingidentity test passed on main only because its trigger (lifetime-mismatch) is thrown by code that reachedAgentStateErrorthrough the externalized./index.js.Tests (both fail against main's dist — swapping main's
distinto the workspace givesAgentContractError is defined in 3 dist filesandsqliteRevisionError.instanceOfStateError: false):packages/rsc-runtime/tests/state-packaging.test.ts(integration pool) now walks the dist import graph per entry (tests/support/dist-graph.ts): every dist file is reachable from some public entry (so a chunk the walker's import parser missed would fail the test, not silently weaken it); root and./plugingraphs contain no file that definesAgentStateError/AgentNoticeError(by definition file, whatever the quoting) and no kernel/ledger/sqlite identifier, and./pluginstays Effect-free; every file that mentionsnode:sqlite/DatabaseSyncis reachable from./state/sqliteand from no other entry (by reachability, so Rspack may move the sqlite code into its own chunk without breaking the test); every error class declared insrc(this.name = '…Error') is defined exactly once across the dist (occurrences counted, not files), andAgentStateError's file is in the graph of./state,./state/sqlite,./mount,./lineage,./notices; a child process (--conditions=react-server,NODE_OPTIONSstripped so a host preload cannot skewmoduleLoadList) imports all nine entries, assertsprocess.moduleLoadListhas noNativeModule sqlitebefore./state/sqliteand has it after,root.AgentRequestError === plugin.AgentRequestError, and errors thrown by the sqlite entry (lifetime-mismatch, and the formerly forkedinvalid-input) and the mount entry areinstanceof./state'sAgentStateError.packages/rsc-runtime/tests/packed-entry-identity.test.ts(packed pool, registered inrstest.integration-tests.ts:152) installs the release tarball with npm, runs the same static graph checks against the installeddist(reachability, one definition per class, sqlite confinement — the probe alone would not see a deferredimport('node:sqlite')), and repeats the probe through theexportsmap (@agent-bundle/runtime,/plugin,/flight/server,/state,/state/sqlite,/notices,/notices/inbox-route,/mount,/lineage), plus: no@modelcontextprotocol/sdkin the installed manifest or declarations, no*peer range,@rspack/coreoptional, andimport.meta.resolve('@agent-bundle/runtime/package.json')resolves.lineagehas no cheap runtime trigger for anAgentStateError(itsinstanceofis inside idempotency-conflict redelivery handling), so it is covered structurally: one definition in the dist, and that file is in the lineage entry's graph.Rslib 0.23 vs 1.0: the config uses only
lib[].source.entry,bundle,dts,format,syntax, and top-leveloutput— the same shape on both; nothing here needs a follow-up forbuild/rslib-1.P3 — manifest hygiene (
packages/rsc-runtime/package.json):71—"./package.json": "./package.json"export (attw: 🟢 JSON on every resolution mode).:83-87—@rspack/coremarkedpeerDependenciesMeta.optional. No runtime entry imports it (packeddist/*.jsexternals:react,react-server-dom-rspack/{server,client}.node,effect,zod,flare-redact,rsc-markdown-stream,@modelcontextprotocol/server,node:*); it isreact-server-dom-rspack's build-plugin peer, whichagent-bundlesupplies at build time. This is exactly the AB7014 guidance for a required peer nothing imports. Measured against real installs (npm 12 and pnpm 11, before/after tarballs in fresh consumers): today the flag changes nothing observable —react-server-dom-rspack@0.1.0still declares the same@rspack/core ^2.2.0-0as a required peer, so both package managers auto-install@rspack/core@2.2.2either way with byte-identical lockfile entries and no warnings; the only visible difference ispnpm peers check(withauto-install-peers=false) no longer listing the runtime as a wanter. The runtime provably loads every entry, including./state/sqlite, with@rspack/coreabsent (npm install --legacy-peer-deps). The consumer-facing win lands whenreact-server-dom-rspackmarks its peer optional too — worth a follow-up there, not here.react/react-domstay exact19.2.8and required — deliberate, not accidental: the Flight wire format is pinned to the React build (docs/plans/2026-08-14-rsc-agent-runtime-demo-design.md:364,agent-bundlepins the same), andreact-server-dom-rspack'sclient.node(root entry) andserver.node(./flight/server) bothrequire("react-dom")at import time. No*ranges.rsc-markdown-streamstays^0.1.0(:93) with thepnpm-workspace.yamlworkspace:*override, per the read ofdocs/preview-packages.mdand the harness:scripts/run-packed-tests.mjsandtests/support/shared-pack.tspack withnpm pack, which does not rewriteworkspace:protocols, so aworkspace:*specifier would ship verbatim in the harness tarball and fail the consumer install withEUNSUPPORTEDPROTOCOL; the AB7015 rule documents the same (workspace:counts as a registry specifier only under a pnpm/Yarn/Bunprepacklifecycle). The override already gives the workspace the linked package; the published range is what consumers need.pnpm lint:release(rootpackage.json) now runsattw --pack --profile esm-onlyandscripts/check-declaration-imports.mjson the packed runtime as well — build: gate releases on attw + declaration-import check; manifest hygiene for agent-bundle, rsc-markdown-stream, create-agent-bundle (#566 §2) #568 added that gate for the other three packages; the runtime is the fourth (48 packed declarations, 44 reachable from 9 export entries, 0 errors, 0 warnings).preview-packages.mdx(en + zh) lists it.agent-bundle prepackapplies to agent-bundle projects, so the rules were checked by hand against the packed tarball — everydependenciesentry is imported by packed JS or referenced by a packed.d.ts; every specifier is a registry version/range; the one peer nothing imports is now optional.Docs
No page under
website/docs/en/**lists the runtime's dependencies, install size, or entry layout (installation.mdxnames the package;preview-packages.mdxmentions the renderer dependency, unchanged).guide/authoring/mcp.mdx(en + zh) gains a paragraph naming the lowered result typeMcpCallToolResult/McpContentBlockand its assignability to both SDK lines'CallToolResult;packages/rsc-runtime/README.md:158says the same.docs/preview-packages.md:70listed the runtime's peers asreact,react-dom,@rspack/core— now says the last is optional. A repo-wide sweep found no consumer outsidepackages/rsc-runtimenaming adist/path of the runtime (everything goes through theexportssubpaths), so the extra shared chunks are invisible to them.pnpm docs:site:buildpasses (TypeDoc compiles the public API, which reaches the new exported types).Gates
On the final state:
pnpm build,pnpm typecheck(plusexamples/rsc-agent-runtime'stsc, the 1.x consumer),pnpm lint,pnpm test:unit(3589 passed),state-packaging.test.ts(7/7),packed-entry-identity.test.tsthroughscripts/run-packed-tests.mjs(1/1),pnpm docs:site:build. On the previous push:pnpm test:integration:run(1065 passed),pnpm test:packed(11 files, 28 passed, 1 skipped). A compile check (in/tmp, against the workspace types) confirmedattachMcpStructuredContentround-trips a 1.xCallToolResult, a 2.xCallToolResult, andMcpCallToolResult, and thatMcpCallToolResultis assignable to both SDK lines; its negative control (const bad: McpCallToolResult = legacy) fails as expected.CI on
f22c2e9had one red job,Release gates (Node 22.19):packages/workbench/tests/packed-release.e2e.test.ts› "foreground outage ledger quiet fence" — the first/api/project/sessionprobe 9 ms into a Workbench restart gotnet::ERR_SOCKET_NOT_CONNECTEDwherepacked-outage-ledger.ts:167tolerates onlyERR_CONNECTION_REFUSED/ERR_CONNECTION_RESET; the next eleven probes wereREFUSEDand recovery returned 200. This PR does not touchpackages/workbench, and the runtime's own packed test passed in that same job. It recurred on the first merged head (e9f041967, run 33936651225) whileRelease gatespassed onmainand four other PRs in the same window;closeChildsends SIGTERM (graceful), so the browser's first retry races the server closing its keep-alive sockets and Chromium reports RESET (tolerated since 7eefeae) or SOCKET_NOT_CONNECTED (not yet) by sub-millisecond ordering — nothing the runtime's dist layout can influence. Onegh run rerun --failedof 33936651225 passed with no code change. Follow-up, out of this PR's scope: addnet::ERR_SOCKET_NOT_CONNECTEDtodownServerProbeCodesinpackages/workbench/tests/support/packed-outage-ledger.ts:167.Self-review
Reviewer:
change-risk-reviewersubagent, modelgpt-5.6-sol-medium, diff vsorigin/main; plus ageneralPurposetest-robustness review of the new test files.attachMcpStructuredContentinput narrowed from SDK 1.xCallToolResulttoMcpCallToolResult— should-fix. A 1.x-typed result has_meta: { [x: string]: unknown; … }, not assignable toJsonObject; external callers would stop compiling. Fixed: the function is generic<T extends CallToolResult>(result: T, value: unknown): T(project-mcp.ts:215-228); compile check with both SDK lines and the package's own type passes, negative control fails.errorClassDefinitionscounted files, not occurrences — two copies in one chunk would pass. Fixed: one entry per match (dist-graph.ts), assertion unchanged (toHaveLength(1)).import()kept; and both tests now assertunreachedFiles(sources)is empty — Rslib emits no unreferenced chunk, so a missed edge form surfaces as an unreached file instead of a silently smaller closure.state/sqlite.jsby name — a legitimate Rspack split into a private chunk would fail it. Fixed: every file mentioningnode:sqlite/DatabaseSyncmust be reached by./state/sqliteand no other entry (entriesReaching).import('node:sqlite')in another entry would not run during the probe. Fixed: the installeddistgets the same reachability, definition-count, and confinement checks.this.name = 'AgentStateError'marker. Fixed: the root and plugin closures must not contain the definition file ofAgentStateError/AgentNoticeError, found quote-agnostically.NODE_OPTIONS— a host preload importingnode:sqlitewould fail the test spuriously. Fixed:probeEnvironment()strips it in both tests.node:sqliteisolation of the shipped dist, in-repo consumers of the renamed types, changeset, docs parity.Second pass (
change-risk-reviewer,gpt-5.6-sol-medium, after the fixes above) — verified finding 1's fix; two new findings:attachMcpStructuredContent— should-fix. Fixed:examples/rsc-agent-runtime/tests/mcp-lowering.test.tsx(the repo's one 1.x-typed consumer) annotates a lowered result and itsattachMcpStructuredContentround-trip as the 1.x SDK'sCallToolResult; the example'stsc(CI "Examples check") fails if either narrows again — verified by reintroducing the narrowing, rebuilding the runtime declarations, and watching the example typecheck fail with TS2345 on that line.McpCallToolResult/McpContentBlockundocumented on the website — must-fix per AGENTS.md docs parity. Fixed: paragraph inwebsite/docs/{en,zh}/guide/authoring/mcp.mdxplus the README line;pnpm docs:site:buildgreen (parity check included).Third pass (
change-risk-reviewer,gpt-5.6-sol-medium, on the final head): no findings in any category — docs match source, zh paragraph faithful, example test correct, no remaining merge risk.